Add QOI compression#670
Conversation
Marc-Andre Lureau (elmarco)
force-pushed
the
qoi
branch
from
de777c8 to
b1f161a
Compare
|
Custom codecs not supported by the Microsoft RDP implementation have rarely been used, but since we're building both the client and server in IronRDP, we're free to experiment! QOI is an excellent choice, it is very simple, and easier to optimize due to its low complexity. IIRC QOI uses RGB, not YUV like most codecs. Color conversion and chroma subsampling alone can take up a lot of the processing time if not properly optimized. There is just one custom codec extension I've seen in the past: JPEG in RDP. XRDP supports it in the server, and FreeRDP supports it in the client as an optional build feature. JPEG has many advantages: it is by far the most widely available image codec, but it is also one for which the most effort has been put into optimizing the implementation (think libjpeg-turbo). For an RDP web client, this also makes it possible to leverage built-in browser support for JPEG decoding, which is a huge advantage over porting and optimizing a custom codec in WASM. In the past, we've designed our own remote desktop protocol (Wayk Now) at Devolutions, and we experimented with many different ways to deal with the codecs more efficiently than RDP. We had GFWX ported in Rust: https://github.com/Devolutions/gfwx-rs GFWX, unlike QOI, is a simplified wavelet codec, which would make it closer to RemoteFX. It is generic and can accept a wide variety of YUV color formats (RGB wouldn't perform well, you're better off doing the YUV conversion to avoid the color channel correlation issue of RGB). It is one of the rare codecs that would enable really nice things like lossless encoding in which you could trim part of the encoded output for lossy encoding. In our implementation, we've used a reversible color conversion (YCoCg-R) due to its reduced complexity (it's only integer operations, not floating point). Unfortunately, we found out JPEG still performed better than GFWX. In theory GFWX offers ways to beat JPEG, but libjpeg-turbo is just hard to beat. If you're curious about the color transformations we've experimented with, we still have them here: I'm open to experimenting with various custom codecs, the ones in RDP are good, but not necessarily special or great. We can definitely beat the standard protocol if we go custom :) |
|
Marc-André Moreau (@awakecoding) I added some jpegturbo benchmarks above. For lossless, desktop usage, QOI is very good - jpeg is far behind. Performance with wasm should be ok (rust-qoi compiles for wasm target) We used to have some heuristics in Spice server to detect video zones, and used jpeg aggressively iirc (when not using whole frame gpu video encoding). |
|
I’m impressed by the characteristics of this codec, especially given how simple the compression format is. As a custom codec, it seems to be a solid alternative to JPEG:
And the Rust implementation is not even optimized yet. For the reasons explained by Marc-André, I don’t think we can beat JPEG in the browser (yet?) even with WASM, but I suspect it’s better than the current implementation which does not rely on the native JPEG implementation of the browser anyway (RLE / RDP 6.0 compression codec → RGB → JavaScript Image object). Definitely does not hurt to use QOI. If we really want to squeeze performance using JPEG, we could consider that too in the future though. Obviously the limitation is that only IronRDP ↔ IronRDP will be supported, but I’m interested in seeing this as an option similar to how JPEG is an option for FreeRDP ↔ XRDP. By the way, do you have an idea how good it performs on the client side (decoding)? Good opportunity to get some some basic benchmarks merged too. |
Marc-Andre Lureau (elmarco)
force-pushed
the
qoi
branch
from
b1f161a to
e893cb6
Compare
Marc-Andre Lureau (elmarco)
force-pushed
the
qoi
branch
6 times, most recently
from
78bd227 to
68571b4
Compare
Marc-Andre Lureau (elmarco)
changed the title
|
Benoît Cortier (@CBenoit) I think we should be good to start reviewing this series. I can split various preliminary patches in different MR if you prefer? |
Sounds great! Yes, I would prefer multiple MRs. I’ll do my best to be as responsive as possible! |
Marc-Andre Lureau (elmarco)
force-pushed
the
qoi
branch
from
68571b4 to
0e5616b
Compare
Marc-Andre Lureau (elmarco)
force-pushed
the
qoi
branch
from
0e5616b to
b3f5b2a
Compare
Marc-Andre Lureau (elmarco)
force-pushed
the
qoi
branch
from
b3f5b2a to
39c30d9
Compare
Marc-Andre Lureau (elmarco)
force-pushed
the
qoi
branch
from
39c30d9 to
1138bac
Compare
|
Hi Marc-Andre Lureau (@elmarco) |
|
Benoît Cortier (@CBenoit) yes, this has been pending for too long. I was waiting for qoi-rust maintainer. To unblock the situation, I decided to fork qoi and released it on crates.io. The qoi-rust maintainer now said he will resume his work, but we can already make progress on this PR without waiting for him. thanks |
|
Sounds great! I’m starting the review. First, this commit is not following the convention: benches:: fix could not find time in tokio |
There was a problem hiding this comment.
issue: The commit feat(session): add apply_rgb24() to apply non-inverted bitmap does not add any feature by itself. It does not change any existing behavior, nor it exposes any new item. Looks like internal groundwork for one of the next commit. Should be refactor(session). Note that we are using the commits for auto-generating the changelog, and the idea here is that this commit should not be part of the changelog, or at least it should not appear as adding a new consumer-facing feature (in fact we exclude refactor commits from the changelog).
Benoît Cortier (CBenoit)
left a comment
Benoît Cortier (CBenoit)
left a comment
There was a problem hiding this comment.
issue: Invalid commit type for feature(server)!: replace with_remote_fx option with codecs, use feat
suggestion: Could be a follow up PR, but I think that the crate ironrdp-cfg may be a better place for client_codecs_capabilities and server_codecs_capabilities. It feels off to me to have CLI-related code in the protocol layer.
|
I’m done with the review. As always, excellent job! |
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Add an optional "flip" argument for inverting bitmaps. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Teach the server to support customizable codecs set. Use the same logic/parsing as the client codecs configuration. Replace "with_remote_fx" with "codecs". Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
The Quite OK Image format ([1]) losslessly compresses images to a similar size of PNG, while offering 20x-50x faster encoding and 3x-4x faster decoding. Add a new QOI codec (UUID 4dae9af8-b399-4df6-b43a-662fd9c0f5d6) for SetSurface command. The PDU data contains the QOI header (14 bytes) + data "chunks" and the end marker (8 bytes). Some benchmarks showing interesting results (using ironrdp/perfenc) Bitmap: 74s user CPU, 92.5% compression RemoteFx (lossy): 201s user CPU, 96.72% compression QOI: 10s user CPU, 96.20% compression Note: the "qoicoubeh" crate is my own fork of "qoi-rust" project. The plan is to switch back to it as soon as the maintainer resume its activites (aldanor/qoi-rust#14). [1]: https://qoiformat.org/ Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Add a new QOIZ codec (UUID 229cc6dc-a860-4b52-b4d8-053a22b3892b) for SetSurface command. The PDU data contains the same data as the QOI codec, with zstd compression. Some benchmarks showing interesting results (using ironrdp/perfenc) QOI: 10s user CPU, 96.20% compression QOIZ: 11s user CPU, 99.76% compression Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Marc-Andre Lureau (elmarco)
force-pushed
the
qoi
branch
from
1138bac to
d2e56e8
Compare
Benoît Cortier (CBenoit)
left a comment
Benoît Cortier (CBenoit)
left a comment
There was a problem hiding this comment.
LGTM! Thanks!!
Benoît Cortier (CBenoit)
merged commit 87df67f
into
Devolutions:master
| let res = zctxt | ||
| .compress_stream2( | ||
| &mut outb, | ||
| &mut inb, | ||
| zstd_safe::zstd_sys::ZSTD_EndDirective::ZSTD_e_flush, | ||
| ) | ||
| .map_err(zstd_safe::get_error_name) | ||
| .unwrap(); | ||
| if res != 0 { | ||
| return Err(anyhow!("Failed to zstd compress")); | ||
| } |
There was a problem hiding this comment.
According to the doc of the function compress_stream2:
An lower bound for the amount of data that still needs to be flushed out.
This is useful when flushing or ending the frame: you need to keep calling this function
until it returns 0.
Why is it fine to call it only once in our case?
| zstd_safe::zstd_sys::ZSTD_EndDirective::ZSTD_e_flush, | ||
| ) | ||
| .map_err(zstd_safe::get_error_name) | ||
| .unwrap(); |
There was a problem hiding this comment.
question: Why is it fine to call unwrap at this place?
There was a problem hiding this comment.
you are right, I looked over it as it doesn't happen in my tests. I'll fix it.
3 participants
I recently found the QOI image format (https://qoiformat.org/) and was excited by its potential for desktop remoting.
I hacked up a few things to actually benchmark it compared to the other codecs.
Based on a raw recording of a typical Fedora desktop usage (terminal/web), for 730 4k frames:
None: 5s user CPU, 0% compression
Bitmap: 74s user CPU, 92.5% compression
RemoteFx (lossy): 201s user CPU, 96.72% compression
QOI: 10s user CPU, 96.20% compression
UPDATE:
jpeg-turbo 80% 4:2:2: 16s user CPU, 97.92% compression
jpeg-turbo 95% 4:4:4: 22s user CPU, 96.13% compression
jpeg-turbo lossless: 135s user CPU, 86.06% compression
UPDATE2:
tile diff+QOI+zstd stream long window: 11s user CPU, 99.76% compression!!
There is still a lot of various optimizations to be desired regardless of the codecs, but this will hopefully help. Also, we should start thinking how QOI could be adapted for streaming.